Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Docs: Submit documentation outlining proposed method of upstream merges #58

Merged

Conversation

digicontributer
Copy link

Hey guys I thought it might be prudent to have some documentation outlining the process we should adhere to when doing any upstream merges for bitcoin.

Hoping we can use this as a starting ground for bringing in the latest Bitcoin 0.22x changes and am open to any feedback or things I may have missed.

@ChillingSilence
Copy link

cACK, Seems close enough to the time I was on the video-call with you and Jared doing it. Thanks @digicontributer .

Is it worth mentioning on line 30 about avoiding the copyright for Bitcoin devs, coz it's only mentioned on Line 70.

Perhaps after step 8 or 9, make mention of running through the unit tests also?

@digicontributer
Copy link
Author

Is it worth mentioning on line 30 about avoiding the copyright for Bitcoin devs, coz it's only mentioned on Line 70.

Perhaps after step 8 or 9, make mention of running through the unit tests also?

Good additions I'll add them in today.

Copy link
Member

@gto90 gto90 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a solid start @digicontributer ! I left a few comments but they aren't critical to the review.

cACK


4. Commit each separate name swap as its own commit. The idea is to break down the sheer # of name changes in a documented, easy-to-follow process. Not following a consistent naming convention throughout the code easily breaks things and can cause hours of compiler errors later on.

5. Rename all filenames including the text "bitcoin" to "digibyte." This is easily done with finder on Mac. Open folder, use option + over arrow button, then command A to select all. Do this 10 times to open all folders and select all files in the entire code base. Then rename and replace all files containing bitcoin -> digibyte. If all file names are not consistent and everything changed you will get compiler errors later on.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure this is necessary right now, but Mac OS Automator could be used to automate the renaming of the files rather than by hand. We could also create a script to do this, or perhaps we leave it up to the contributors?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I definitely agree that a script could be useful here, I don't have any experience with Mac OS Automater so can't help there.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The makefiles at this step will be looking for the wrong file names if they aren't yet updated.

Copy link
Member

@ycagel ycagel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line 13: Spelling of Relelvent should be Relevant

Copy link
Member

@ycagel ycagel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cACK

Copy link

@ChillingSilence ChillingSilence left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK from me.
Looks good @digicontributer , thanks again. No doubt we can review it after a few of these have been done for further updates if required

1. Completely build latest Bitcoin Core from source all the way to deployment to ensure you have the proper build environment configured. Follow latest BTC dev build environment changes & thoroughly read release notes.

git clone https://github.com/bitcoin/bitcoin.git
cd bitcoin ./autogen.sh ./configure make

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it worth mentioning the depends/ dir and specifying build of those, rather than relying on --incompatible-bdb etc?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, agreed but I think that might be worth its own PR as we can potentially cover the other documentation while at it (gitian, native, etc).

@digicontributer digicontributer merged commit 18b060f into DigiByte-Core:feature/8.22.0 Jul 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants